pandaproxy: fix unmatched-route 404 body shape#30417
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR standardizes the JSON error envelope returned for unmatched routes (unregistered path/method) in Pandaproxy-backed HTTP servers (REST proxy and Schema Registry) by registering a Seastar default handler that emits Pandaproxy’s canonical {"error_code": <int>, "message": "..."} body.
Changes:
- Add a
default_404_handlerthat returns the Pandaproxy/SR-compatible 404 envelope for unmatched routes. - Register the default handler in
pandaproxy::server::start()so it applies to both REST proxy and Schema Registry. - Add unit + rptest coverage to lock the 404 body shape end-to-end for both ports.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/rptest/tests/schema_registry_test.py | Adds an rptest asserting SR unmatched-route 404 returns error_code envelope. |
| tests/rptest/tests/pandaproxy_test.py | Adds an rptest asserting REST proxy unmatched-route 404 returns error_code envelope. |
| src/v/pandaproxy/test/default_404_handler.cc | Adds a C++ unit test validating the default handler’s JSON envelope. |
| src/v/pandaproxy/test/BUILD | Registers the new C++ unit test target. |
| src/v/pandaproxy/server.h | Stores the default handler to ensure it outlives Seastar routes. |
| src/v/pandaproxy/server.cc | Registers the default handler on the Seastar routes table during startup. |
| src/v/pandaproxy/default_404_handler.h | Introduces the default 404 handler implementation. |
| src/v/pandaproxy/BUILD | Exposes the new handler header in the pandaproxy library. |
pgellert
previously approved these changes
May 11, 2026
Comment on lines
+40
to
+51
| ss::future<std::unique_ptr<ss::http::reply>> handle( | ||
| const ss::sstring&, | ||
| std::unique_ptr<ss::http::request>, | ||
| std::unique_ptr<ss::http::reply> rep) final { | ||
| auto ec = make_error_condition(reply_error_code::not_found); | ||
| json::error_body body{.ec = ec, .message = ss::sstring{ec.message()}}; | ||
| rep->set_status(ss::http::reply::status_type::not_found); | ||
| rep->write_body( | ||
| _mime_type, pandaproxy::json::rjson_serialize_str(std::move(body))); | ||
| return ss::make_ready_future<std::unique_ptr<ss::http::reply>>( | ||
| std::move(rep)); | ||
| } |
Contributor
There was a problem hiding this comment.
nit: it would be nice to move this out of the header file into a .cc implementation file. It's a small file, that's perhaps unlikely to get more implementation, so I think it's a non-blocking nit in this case, but we should for new codem generally prefer to:
- move as much of the implementation into
.ccfiles - prefer 1 bazel target per
.ccfile unless there's a reason to merge it into an existing target (e.g. cyclical build dependencies)
Comment on lines
+262
to
266
| _default_handler = std::make_unique<default_404_handler>( | ||
| ss::sstring{name(_exceptional_mime_type)}); | ||
| _server._routes.add_default_handler(_default_handler.get()); | ||
|
|
||
| _probe = std::make_unique<server_probe>(_ctx, _public_metrics_group_name); |
Contributor
There was a problem hiding this comment.
Do we need to reset the _default_handler pointer during stop()? Does schema_registry::api::restart() work with this?
I think the answer is yes to both, but just double-checking.
The handler emits the {"error_code": 404, "message": "HTTP 404 Not
Found"} envelope that schema registry and REST proxy clients expect,
rather than Seastar's fallback {"message": "Not found", "code": 404}.
Real pandaproxy handlers already emit this shape via
pandaproxy::json::error_body; this class lets the unmatched-route path
do the same once it is wired into pandaproxy::server in a follow-up
commit.
The handler is added with a unit test before any wiring so its body
contract is exercised in isolation. Wiring lands separately so the
server-side change is visible on its own commit.
Wires the new handler so Seastar's routes table dispatches to it for
any (method, path) combination that no registered route matches.
Without this, Seastar's built-in routes::handle() short-circuits with
its own JSON body that uses {"code": 404} instead of the
{"error_code": 404} shape SR clients expect, breaking 404-fallback
paths in external SR client serializers.
The handler is owned by pandaproxy::server via unique_ptr because
Seastar's add_default_handler does not take ownership (per
seastar/include/seastar/http/routes.hh:136). server::start() creates
it; server::stop() resets it after _server.stop() completes, mirroring
how _probe is torn down and making schema_registry::api::restart()
unambiguously safe across stop->start. The field is also declared
before _server so that on destruction the handler outlives the routes
table.
Both REST proxy and schema registry use pandaproxy::server, so this
fixes the body shape for both subsystems with a single registration.
Asserts that GET on a bogus path returns HTTP 404 with the
{"error_code": 404, "message": "..."} envelope clients expect, rather
than Seastar's fallback {"message": "Not found", "code": 404}. Two
assertions, one per port:
- Schema registry (8081) in schema_registry_test.py
- REST proxy (8082) in pandaproxy_test.py
Both subsystems use pandaproxy::server, so the fix in
pandaproxy::server::start() lands on both. Locking in both wire
formats end-to-end is the test that proves the default_404_handler
registration works for both consumers. Removing the
add_default_handler line would regress the body shape and fail both
assertions in CI.
Collaborator
CI test resultstest results on build#84300
|
dotnwat
approved these changes
May 11, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
When a request hits an HTTP path/method that isn't registered with
pandaproxy::server(REST proxy or schema registry), Redpanda returnsSeastar's built-in 404 body
{"message": "Not found", "code": 404}.Schema-registry clients parse a different envelope (
error_coderatherthan
code), so any client logic that inspects 404 bodies breaks onthis shape.
Real pandaproxy handlers already emit the
{"error_code": <int>, "message": "..."}envelope viapandaproxy::json::error_body. The fix adds a default handler thatemits the same shape and registers it on Seastar's routes table in
pandaproxy::server::start(). Both REST proxy and schema registryshare
pandaproxy::server, so the fix lands on both subsystems with asingle registration site.
The three commits:
pandaproxy: add default_404_handler for unmatched-route 404 body shapeNew
default_404_handlerclass plus C++ unit test exercising thebody-shape contract in isolation.
pandaproxy: register default_404_handler in server::start()Wires the handler so Seastar dispatches to it for any unmatched
(method, path)combination. Owned viaunique_ptrbecauseSeastar's
add_default_handlerdoes not take ownership (perseastar/include/seastar/http/routes.hh:136).tests/rptest: cover unmatched-route 404 shape on REST proxy and SRDucktape assertions on both ports (8082 and 8081) lock in the wire
format end-to-end and prove the wiring at the cluster level.
Jira: https://redpandadata.atlassian.net/browse/CORE-16251
Backports Required
Release Notes
Bug Fixes
{"error_code": 404, "message": "..."}for unmatched routes,matching the envelope clients parse.